Adds PATH for macOS #10031

atsansone · 2024-01-11T18:38:00Z

Adds PATH instructions for macOS.
Fixes #9351
Fixes #10015
Fixes #10040
Fixes #10036

flutter-website-bot · 2024-01-11T18:46:20Z

Visit the preview URL for this PR (updated for commit 260add0):

https://flutter-docs-prod--pr10031-fix-9351-7bq7dgok.web.app

atsansone · 2024-01-11T19:37:33Z

@johnpryan : Can you review this as DRE on call this week? The main change is in the Download and Install tab of the macOS install page. The other change is in the Operating system section of the Software requirements section. Thanks!

sfshaza2

Clarified some text.

sfshaza2 · 2024-01-11T20:26:19Z

src/_includes/docs/install/reqs/macos/set-path.md

+You need to add Flutter to the `PATH` environment variable.
+THis section covers the permanent addition of the path to the
+Flutter to your `PATH` environment variable.


Suggested change

You need to add Flutter to the `PATH` environment variable.

THis section covers the permanent addition of the path to the

Flutter to your `PATH` environment variable.

you need to add Flutter to the `PATH` environment variable.

You can add it manually each time you launch a terminal

but it's easier, as shown here,

to add it to your environment so that it's always available.

I'd like to keep this more opinionated and avoid writing about another method that they'll find they need to change later. That's what made the original macOS install page so cumbersome.

I disagree, in this doc it's so opinionated I find it difficult to understand what is required to use Flutter and what's just recommended. Like why .zshenv and not .zshrc? Why would Flutter need me to update GEM_HOME? Have we now messed up the environment for someone who already has Ruby installed? Why are we in the business of telling people to update that env?

The zsh docs say that environment variables should be set in .zshenv because that file is loaded regardless of it being a login or an interactive shell. The other files are not always loaded.

This is also what opinionated means in this case. We're not offering options, we're providing a path to completion. When this is published, we may get issues filed against this and we can address any raised concerns then.

I guess I'm disagreeing that this doc should be opinionated. I suggest we put this page back to:

Install Xcode (link to Xcode in the Mac App Store, or Apple docs)

Install Cocoapods (link to CocoaPods install page)

Put Flutter on your PATH (link to how to put things on your PATH, or maybe a blog post or something like "here's an example of how to do that!")

etc.

As someone who has had to close many GitHub issues and add workarounds to the Flutter tool related to installation and environment issues resulting from our own docs, I'd really prefer not to be in the business of keeping the dependency installation docs up-to-date, or to confuse experienced developers who know how to add executables to their PATH, or make it seem like we have dependencies that we don't (Homebrew).

Once it's installed and built, the flutter tool itself has more up-to-date hints about how to get your environment set up (you have Xcode installed, but you need to accept the license, etc)

src/_includes/docs/install/reqs/macos/set-path.md

johnpryan · 2024-01-11T20:20:14Z

src/_includes/docs/install/compiler/xcode.md

+    then
+        export PATH=\$GEM_HOME/bin:\$PATH
+    fi
+    EOF


We don't need to dictate how the user should add a directory to their path in their .zshenv file. I would prefer give instructions that align more with the Cocoapods Getting Started guide like this:

export GEM_HOME=$HOME/.gem export PATH=$GEM_HOME/bin:$PATH

This meets two requirements:

Not having to explain opening an editor, making the change, then saving it. This section explains how to set the PATH changes permanently, not for the session.

The extra code around this was to keep the PATH sane if the user runs source more than once.

I still think this is unnecessary code to make the user copy-paste. It's also not very readable. I know I spent a while staring at it before I understood what it was doing. But if you said "Add this to your path in .zshenv I would have been able to do it with whichever editor I normally use, and add it in a place that makes sense, perhaps with a comment. We don't need to hand-hold the user through this.

I changed this to opening a text editor and editing the ~/.zshenv file.

johnpryan · 2024-01-11T20:22:43Z

src/_includes/docs/install/compiler/xcode.md

@@ -98,7 +92,26 @@ To [install and set up CocoaPods][cocoapods], run the following commands:
   {{prompt1}} brew install cocoapods
   ```

-Using Homebrew reduces potential issues with chipsets and install permissions.


@jmagman Why do we recommend Homebrew, but the CocoaPods Getting Started guide says to use gem install with the system Ruby installation?

@atsansone It would be great if we can lean more on the CocoaPods documentation here. At the very least, we should try to match what they recommend.

The reason I included Homebrew is that it was included in a previous version of this doc for the reasons stated in the Software requirements: Homebrew handles chipset and permissions issues for you. I would love to avoid:

Sending users to too many other sites.

Needing to cover off more edge cases when something doesn't work.

@johnpryan I wouldn't have suggested Homebrew, I wasn't a reviewer on #9736.

https://docs.flutter.dev/get-started/install/macos/mobile-ios#install-cocoapods currently sounds like Homebrew is a requirement to do Flutter iOS development and it just isn't. cc @christopherfujino

I don't use Homebrew to manage Ruby or CocoaPods. I use the built in Ruby version to macOS and I follow CocoaPod's own installation instructions. For the purpose of this doc I would personally just point them to https://guides.cocoapods.org/using/getting-started.html#installation and call it a day instead of trying to replicate dependency's installation instructions.

I'm also not a fan of Homebrew and skip any steps involving it. Leaning on Cocoapods own installation instructions and the options there seems good 👍

I got lost searching through the git history, but TL;DR we used to recommend brew install cocoapods, but this caused pain for our users, and is NOT the official way to install this, so we standardized on gem install cocoapods across the tool and website as our official recommendation.

For the purpose of this doc I would personally just point them to https://guides.cocoapods.org/using/getting-started.html#installation and call it a day instead of trying to replicate dependency's installation instructions.

This is aligned with what the tool tells users to do if we detect it's either missing or broken: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/macos/cocoapods.dart#L50

Most any explanation I've consulted doesn't recommend the default macOS install of Ruby. We used Homebrew elsewhere, so it seemed better to have one way to install everything instead of one way for this app, another for this other app, etc. Think as a new user: keep it simple.

We refer to Homebrew in about as many pages in the public docs: around 1-2 times each.

Looking at this as a new user, I'd like to recommend simple, consistent ways to handle installs wherever possible. Homebrew offers that option.

Further, the CocoaPods install docs themselves are out of date (referring to bash_profile, for example).

Although, @christopherfujino , your point about what exists in the code makes sense and may need a future revisit.

@jmagman : Understood that CocoaPods is not required to build iOS apps for Flutter. It's required to build Flutter plugins for iOS apps, right? We could make that note, but not in a Getting Started. I would then need to add an additional section or page later to cover that off. What say you?

CocoaPods is required as soon as you add a plugin. I believe in the docs that used to be called out.

It still is. ;)

johnpryan · 2024-01-11T20:25:26Z

src/_includes/docs/install/compiler/xcode.md

+
+    ```terminal
+    $ source ~/.zshenv
+    ```


I think we should just leave this out, we could just say "Restart your terminal".

If you're like me, having to restart 5-6 terminals would be... unpleasant. This command is a shorter way to do this.

Why? Don't you just need to close the terminal window and open a new one?

I run Tmux and have multiple tabs open in my Terminal app.

If someone is advanced enough to use Tmux, they should be advanced enough to know how to handle this scenario

johnpryan · 2024-01-11T20:27:11Z

src/_includes/docs/install/reqs/macos/set-path.md

+    then
+        export PATH=\$FLUTTER_HOME/bin:\$PATH
+    fi
+    EOF


Same comment as above, this code is overcomplicated.

The users need only copy and paste. They don't need to interpret this in any way.

It's hard to read, I would just describe the code they need to add to their shell initialization (whether it's Zsh, Bash, or something else)

That's the thing: It should be zsh across the board at this version of the OS. I don't want to assume a particular level of knowledge or familiarity with all areas involved.

drive by comment, @atsansone you're right, users will copy paste shell snippets to their config. also, many tools auto append. this leads to users having very long, incomprehensible config files and obscure runtime shell environments that we are not easy for my team to debug.

johnpryan · 2024-01-11T20:51:17Z

src/_includes/docs/install/flutter/download.md

+{% include docs/install/reqs/windows/set-path.md terminal=terminal target=target %}
+{% when 'macOS' %}
+{% include docs/install/reqs/macos/set-path.md terminal=terminal target=target %}
+{% endcase %}


I don't know if it's just me but this structure is hard to review with all of these includes. I've noticed that the size of the _includes directory is increasing and I'm not sure that's a great trend for maintainability overall. If there are places where we can just add the docs in-line I think that would be better in the long term.

Includes are great for reusable snippets of instructions but might have diminishing returns if we use them too much.

It can be on first pass, but future updates may be easier. Most of these types of pages get a lot of reuse as most (~75%) of the content is the same for all 4 development platforms.

It's really hard for me to follow what's going on. I have to jump around from file to file rather than just read it in-line.

johnpryan · 2024-01-11T20:53:20Z

src/_includes/docs/install/flutter/vscode.md

@@ -128,3 +133,5 @@
   1. Restart VS Code.

 [development tools prerequisites]: #development-tools
+[Visual Studio Code]: https://code.visualstudio.com/docs/setup/mac
+[Flutter extension for VS Code]: https://marketplace.visualstudio.com/items?itemName=Dart-Code.flutter


Unused link?

src/_includes/docs/install/reqs/macos/base.md

src/_includes/docs/install/reqs/macos/set-path.md

src/_includes/docs/install/reqs/macos/zsh-config.md

atsansone · 2024-01-12T20:35:27Z

@johnpryan , @sfshaza2 , @christopherfujino , @jmagman : Thank you all for your input on this PR. I believe we've had some constructive conversations that will produce a better docs page. I will make the updates to remove Homebrew, simplify PATH instructions, simplify CocoaPods install instructions, and clarify per-session vs. permanent PATH changes in my next commit. This is an FYI and the next commit will be a little later today.

Again, my thanks for your time and assistance!

atsansone · 2024-01-16T16:38:12Z

@johnpryan : PTAL.

johnpryan

Looking better, but there's still some feedback that needs to be addressed.

johnpryan · 2024-01-16T18:43:36Z

src/_includes/docs/install/compiler/xcode.md

+   if [[ $PATH != *"$GEM_HOME"* ]]
+   then
+       export PATH=$GEM_HOME/bin:$PATH
+   fi


Please see my comment about this:

#10031 (comment)

I understand your point, but don't agree with it. I stand by the change.

I agree with John here. In all of my Unix life of 40+ years, I have NEVER defined the GEM_HOME environment variable. Please simplify.

johnpryan · 2024-01-16T18:45:51Z

src/_includes/docs/install/reqs/macos/base.md

+and software requirements.
+
+All of the commands in this guide have been tested on a MacBook Pro M1
+running macOS 14.2.1.


Take this paragraph out.

I would rather not. It heads off certain questions.

johnpryan · 2024-01-16T18:46:45Z

src/_includes/docs/install/reqs/macos/set-path.md

+   if [[ $PATH != *"$FLUTTER_HOME"* ]]
+   then
+       export PATH=$FLUTTER_HOME/bin:$PATH
+   fi


This section is too much. See my comment here: #10031 (comment)

I understand your point, but don't agree with it. I stand by the change.

johnpryan · 2024-01-16T18:47:31Z

src/_includes/docs/install/reqs/macos/zsh-config.md

+<details markdown="1">
+<summary><strong>To verify your shell configuration, expand this section</strong></summary>
+
+Like most UNIX-like operating system, macOS can support multiple shells,


Take these instructions out, they don't belong in the Flutter docs and they will increase our maintenance burden in the long term.

johnpryan · 2024-01-16T18:48:03Z

src/_includes/docs/install/reqs/macos/zsh-config.md

+   the default macOS shell, run the following commands.
+
+    ```terminal
+    $ which zsh; dscl . -read ~/ UserShell


I think this entire section is misguided and doesn't need to be added to our docs, see my comment above.

Co-authored-by: John Ryan <ryjohn@google.com>

domesticmouse · 2024-01-22T20:19:26Z

src/_includes/docs/install/compiler/xcode.md

@@ -71,34 +71,33 @@ With Xcode, you can run Flutter apps on an iOS device or on the simulator.

 {% endif %}

-### Install CocoaPods
+### Install CocoaPods (Optional)


Optional feels wrong. If required?

sfshaza2 · 2024-01-22T21:18:28Z

I agree with many of the comments against this PR. It seems unnecessarily complex. I doubt many of our users are using Tmux. And we do want to avoid Homebrew, at least for now. I also don't want to get into the habit of telling them which shell to use. I still use Borne.

sfshaza2 · 2024-01-22T21:30:45Z

By the way, regarding "presumes" vs "assumes":

"Assumes" is much more consistent with our docs
It's easier for non-native English speakers to understand

@domesticmouse

Per @domesticmouse review. Removing optional.

Adds PATH instructions for macOS. Fixes flutter#9351 Fixes flutter#10015 Fixes flutter#10040 Fixes flutter#10036 --------- Co-authored-by: John Ryan <ryjohn@google.com>

atsansone added the review.copy Awaiting Copy Review label Jan 11, 2024

atsansone requested review from parlough, sfshaza2, khanhnwin, domesticmouse and MaryaBelanger as code owners January 11, 2024 18:38

atsansone requested a review from johnpryan January 11, 2024 19:34

atsansone added review.tech Awaiting Technical Review and removed review.copy Awaiting Copy Review labels Jan 11, 2024

sfshaza2 reviewed Jan 11, 2024

View reviewed changes

johnpryan requested changes Jan 11, 2024

View reviewed changes

atsansone mentioned this pull request Jan 13, 2024

Install page issues for developing on iOS #10036

Closed

1 task

atsansone force-pushed the fix-9351 branch from 123ff7e to 0a90b06 Compare January 16, 2024 17:14

johnpryan requested changes Jan 16, 2024

View reviewed changes

atsansone and others added 8 commits January 18, 2024 16:18

Added PATH instructions for macOS install

d4d7ef6

fixing links

c9ac009

Apply suggestions from code review from @johnpryan

efc4bec

Co-authored-by: John Ryan <ryjohn@google.com>

Update vscode.md

3956e4b

Updated per review feedback

02a1e6b

Added fix for flutter#10040

7d9215f

Added 2nd fix for flutter#10040

66227c8

Updated notice

35b1737

atsansone force-pushed the fix-9351 branch from a5851f7 to 3a9d5e6 Compare January 18, 2024 22:19

Update zsh and PATH sections

bf1bb02

atsansone force-pushed the fix-9351 branch from 3a9d5e6 to bf1bb02 Compare January 18, 2024 22:31

Merge branch 'main' into fix-9351

5a98236

atsansone and others added 2 commits January 19, 2024 11:03

Updated per @johnpryan feedback

60962b3

Merge branch 'main' into fix-9351

698fd89

johnpryan approved these changes Jan 19, 2024

View reviewed changes

atsansone added review.copy Awaiting Copy Review and removed review.tech Awaiting Technical Review labels Jan 19, 2024

atsansone assigned sfshaza2 Jan 19, 2024

atsansone requested a review from sfshaza2 January 20, 2024 02:13

Merge branch 'main' into fix-9351

efb4237

domesticmouse approved these changes Jan 22, 2024

View reviewed changes

Update xcode.md

260add0

Per @domesticmouse review. Removing optional.

atsansone merged commit 9c25dca into flutter:main Jan 22, 2024
9 checks passed

atsansone added st.RFM Ready to merge or land and removed review.copy Awaiting Copy Review labels Apr 15, 2024

-You need to add Flutter to the `PATH` environment variable.
-THis section covers the permanent addition of the path to the
-Flutter to your `PATH` environment variable.
+you need to add Flutter to the `PATH` environment variable.
+You can add it manually each time you launch a terminal
+but it's easier, as shown here,
+to add it to your environment so that it's always available.

Adds PATH for macOS #10031

Adds PATH for macOS #10031

Conversation

atsansone commented Jan 11, 2024 • edited Loading

flutter-website-bot commented Jan 11, 2024 • edited Loading

atsansone commented Jan 11, 2024

sfshaza2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman Jan 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atsansone Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

johnpryan Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmagman Jan 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atsansone commented Jan 12, 2024

atsansone commented Jan 16, 2024

johnpryan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfshaza2 commented Jan 22, 2024 • edited Loading

sfshaza2 commented Jan 22, 2024

atsansone commented Jan 11, 2024 •

edited

Loading

flutter-website-bot commented Jan 11, 2024 •

edited

Loading

jmagman Jan 12, 2024 •

edited

Loading

atsansone Jan 11, 2024 •

edited

Loading

johnpryan Jan 11, 2024 •

edited

Loading

jmagman Jan 11, 2024 •

edited

Loading

sfshaza2 commented Jan 22, 2024 •

edited

Loading